Skip to content

[WIP] Proposal: replace asyncio with anyio#165

Draft
valgarf wants to merge 2 commits intographql-python:mainfrom
valgarf:main
Draft

[WIP] Proposal: replace asyncio with anyio#165
valgarf wants to merge 2 commits intographql-python:mainfrom
valgarf:main

Conversation

@valgarf
Copy link

@valgarf valgarf commented Apr 2, 2022

This PR replaces almost all asyncio usage with anyio, making graphql-core run with asyncio and trio.

This is only a proposal. It is implemented completely and tests run with asyncio and trio, but the PR also includes a few decisions that should probably be discussed before adopting the changes.

Current state:

  • tox runs successfully for python 3.7, 3.8, 3.9, and 3.10 (python 3.6 is currently broken on my machine, cannot run the tests with it)
  • every async test is run in asyncio and in trio
  • performance of the existing async benchmark seems to be unaffected when using the asyncio event loop (see benchmarks below)

The following changes were necessary to use anyio:

  • minium python version had to be increased from 3.6.0 to 3.6.2
  • replacing asyncio functions with their anyio counterpart (e.g. asyncio.sleep -> anyio.sleep)
  • all create_task / ensure_future calls are forbidden in anyio. These code parts were rewritten using anyio task groups.

The following decisions should probably be discussed:

  • exception groups are caught and only the first exception is raised.
  • SimplePubSub / SimplePubSubIterator are unchanged, adapting them to anyio would change their API too much. MemoryObjectBroadcastStream was added to take its place.

ToDo:

  • documentation probably needs to be updated (I did not change anything)
  • run tests & fix issues for python 3.6

I am aware that this is a large change and it comes with (small) downsides,
e.g. a minimum python version of 3.6.2 and some extra effort for the users of SimplePubSub.
But it would allow the use of graphql with the trio event loop.
Having spent countless hours debugging due to some asyncio task that did not close correctly, I would
be extremely happy to have a structured concurrency approach available for graphql.

If you are at all interested in this proposal, please let me know.

Benchmarks before changes: 
============================= test session starts ==============================
platform linux -- Python 3.7.13, pytest-6.2.5, py-1.11.0, pluggy-1.0.0
benchmark: 3.4.1 (defaults: timer=time.perf_counter disable_gc=False min_rounds=5 min_time=0.000005 max_time=5 calibration_precision=10 warmup=True warmup_iterations=100000)
rootdir: /home/stefan/ws/graphql-core, configfile: setup.cfg
plugins: benchmark-3.4.1, cov-3.0.0, asyncio-0.16.0, anyio-3.5.0, timeout-2.1.0, describe-2.0.1
timeout: 100.0s
timeout method: signal
timeout func_only: False
collected 2370 items / 2359 deselected / 11 selected

tests/benchmarks/test_build_ast_schema.py .                              [  9%]
tests/benchmarks/test_build_client_schema.py .                           [ 18%]
tests/benchmarks/test_execution_async.py .                               [ 27%]
tests/benchmarks/test_execution_sync.py .                                [ 36%]
tests/benchmarks/test_introspection_from_schema.py .                     [ 45%]
tests/benchmarks/test_parser.py .                                        [ 54%]
tests/benchmarks/test_validate_gql.py .                                  [ 63%]
tests/benchmarks/test_validate_invalid_gql.py .                          [ 72%]
tests/benchmarks/test_validate_sdl.py .                                  [ 81%]
tests/benchmarks/test_visit.py ..                                        [100%]


--------------------------------------------------------------------------------------------------------- benchmark: 11 tests ----------------------------------------------------------------------------------------------------------
Name (time in us)                                 Min                     Max                    Mean                 StdDev                  Median                   IQR            Outliers         OPS            Rounds  Iterations
----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
test_execute_basic_sync                      679.1020 (1.0)       24,167.3440 (1.08)         795.1533 (1.0)         615.8840 (1.92)         778.2310 (1.0)        110.2390 (1.54)      157;381  1,257.6191 (1.0)        7358           1
test_execute_basic_async                     764.3180 (1.13)      24,417.7180 (1.09)         880.1823 (1.11)        648.0944 (2.02)         832.9540 (1.07)       114.0020 (1.59)       65;265  1,136.1283 (0.90)       6536           1
test_parse_kitchen_sink                    1,361.3850 (2.00)      22,441.8060 (1.0)        1,461.0742 (1.84)        677.6062 (2.11)       1,435.4575 (1.84)        71.7050 (1.0)         4;199    684.4280 (0.54)       3680           1
test_validate_introspection_query          4,038.9070 (5.95)      48,622.5970 (2.17)       4,381.5331 (5.51)      1,802.7149 (5.62)       4,194.7020 (5.39)       111.0915 (1.55)        2;122    228.2306 (0.18)       1241           1
test_validate_invalid_query               37,922.6710 (55.84)     40,026.3080 (1.78)      38,436.2805 (48.34)       365.8637 (1.14)      38,341.9050 (49.27)      330.5452 (4.61)         17;9     26.0171 (0.02)        131           1
test_build_schema_from_ast                40,436.8810 (59.54)     91,369.1450 (4.07)      52,498.9107 (66.02)    19,934.7782 (62.14)     41,902.6470 (53.84)    1,569.7452 (21.89)       29;29     19.0480 (0.02)        123           1
test_build_schema_from_introspection      46,932.1740 (69.11)     87,449.3990 (3.90)      56,032.1161 (70.47)    15,406.7749 (48.02)     47,648.6120 (61.23)    1,379.6577 (19.24)       25;25     17.8469 (0.01)        107           1
test_visit_all_ast_nodes                  57,287.9200 (84.36)     58,812.3470 (2.62)      57,831.8654 (72.73)       320.8184 (1.0)       57,809.7725 (74.28)      468.2680 (6.53)         29;1     17.2915 (0.01)         88           1
test_validate_sdl_document               116,616.9430 (171.72)   119,051.0130 (5.30)     117,596.4405 (147.89)      634.0356 (1.98)     117,494.7540 (150.98)     847.3555 (11.82)        13;0      8.5037 (0.01)         43           1
test_execute_introspection_query         215,637.0560 (317.53)   254,215.4660 (11.33)    222,730.8371 (280.11)   13,984.2738 (43.59)    216,851.9790 (278.65)     877.4840 (12.24)         4;4      4.4897 (0.00)         24           1
test_visit_all_ast_nodes_in_parallel     611,041.8340 (899.78)   617,905.6140 (27.53)    614,241.2548 (772.48)    2,416.0767 (7.53)     613,784.0130 (788.69)   3,670.1400 (51.18)         3;0      1.6280 (0.00)          9           1
----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------

Legend:
  Outliers: 1 Standard Deviation from Mean; 1.5 IQR (InterQuartile Range) from 1st Quartile and 3rd Quartile.
  OPS: Operations Per Second, computed as 1 / Mean
=============== 11 passed, 2359 deselected in 184.28s (0:03:04) ================
Benchmarks after changes: 
============================= test session starts ==============================
platform linux -- Python 3.7.13, pytest-6.2.5, py-1.11.0, pluggy-1.0.0
benchmark: 3.4.1 (defaults: timer=time.perf_counter disable_gc=False min_rounds=5 min_time=0.000005 max_time=5 calibration_precision=10 warmup=True warmup_iterations=100000)
rootdir: /home/stefan/ws/graphql-core, configfile: setup.cfg
plugins: benchmark-3.4.1, cov-3.0.0, anyio-3.5.0, timeout-2.1.0, describe-2.0.1
timeout: 100.0s
timeout method: signal
timeout func_only: False
collected 2516 items / 2504 deselected / 12 selected

tests/benchmarks/test_build_ast_schema.py .                              [  8%]
tests/benchmarks/test_build_client_schema.py .                           [ 16%]
tests/benchmarks/test_execution_async.py ..                              [ 33%]
tests/benchmarks/test_execution_sync.py .                                [ 41%]
tests/benchmarks/test_introspection_from_schema.py .                     [ 50%]
tests/benchmarks/test_parser.py .                                        [ 58%]
tests/benchmarks/test_validate_gql.py .                                  [ 66%]
tests/benchmarks/test_validate_invalid_gql.py .                          [ 75%]
tests/benchmarks/test_validate_sdl.py .                                  [ 83%]
tests/benchmarks/test_visit.py ..                                        [100%]


--------------------------------------------------------------------------------------------------------- benchmark: 12 tests ----------------------------------------------------------------------------------------------------------
Name (time in us)                                 Min                     Max                    Mean                 StdDev                  Median                   IQR            Outliers         OPS            Rounds  Iterations
----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
test_execute_basic_sync                      661.9170 (1.0)       25,781.1220 (1.29)         763.7950 (1.0)         647.2897 (2.59)         727.1705 (1.0)         99.9350 (1.60)       10;310  1,309.2518 (1.0)        7574           1
test_execute_basic_async[asyncio]            789.3590 (1.19)      20,032.9120 (1.0)          867.8673 (1.14)        249.4849 (1.0)          906.9380 (1.25)       120.7202 (1.93)          6;8  1,152.2499 (0.88)       6343           1
test_execute_basic_async_trio[trio]        1,251.8120 (1.89)      25,944.9790 (1.30)       1,409.2772 (1.85)        680.1598 (2.73)       1,395.3320 (1.92)       129.8407 (2.08)        3;228    709.5836 (0.54)       4013           1
test_parse_kitchen_sink                    1,330.0340 (2.01)      23,528.3300 (1.17)       1,424.0817 (1.86)        723.1457 (2.90)       1,402.3740 (1.93)        62.5005 (1.0)         4;185    702.2069 (0.54)       3772           1
test_validate_introspection_query          3,918.3780 (5.92)      47,985.1300 (2.40)       4,178.4954 (5.47)      1,758.7993 (7.05)       4,011.1160 (5.52)        78.7470 (1.26)        2;129    239.3206 (0.18)       1277           1
test_validate_invalid_query               37,027.1740 (55.94)     38,657.1470 (1.93)      37,451.0217 (49.03)       271.5596 (1.09)      37,385.6900 (51.41)      283.0698 (4.53)         30;8     26.7015 (0.02)        135           1
test_build_schema_from_ast                38,402.0660 (58.02)     87,811.6550 (4.38)      50,244.0027 (65.78)    19,427.7732 (77.87)     39,887.3900 (54.85)    1,784.4250 (28.55)       31;31     19.9029 (0.02)        130           1
test_build_schema_from_introspection      44,885.4960 (67.81)     83,229.9070 (4.15)      53,670.0072 (70.27)    14,769.6549 (59.20)     45,680.3725 (62.82)    1,343.1600 (21.49)       26;26     18.6324 (0.01)        112           1
test_visit_all_ast_nodes                  55,416.7260 (83.72)     56,546.7950 (2.82)      55,881.9364 (73.16)       251.1629 (1.01)      55,881.2460 (76.85)      382.7483 (6.12)         31;0     17.8949 (0.01)         91           1
test_validate_sdl_document               112,529.1490 (170.00)   152,931.9640 (7.63)     114,881.9283 (150.41)    5,848.5372 (23.44)    113,915.4470 (156.66)   1,022.5075 (16.36)         1;2      8.7046 (0.01)         45           1
test_execute_introspection_query         210,025.7760 (317.30)   248,757.4280 (12.42)    216,474.8744 (283.42)   12,211.5627 (48.95)    212,063.4750 (291.63)   2,000.2005 (32.00)         3;3      4.6195 (0.00)         24           1
test_visit_all_ast_nodes_in_parallel     610,209.5660 (921.88)   614,215.2950 (30.66)    612,014.3380 (801.28)    1,352.9536 (5.42)     611,614.2970 (841.09)   1,486.0685 (23.78)         3;0      1.6339 (0.00)          9           1
----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------

Legend:
  Outliers: 1 Standard Deviation from Mean; 1.5 IQR (InterQuartile Range) from 1st Quartile and 3rd Quartile.
  OPS: Operations Per Second, computed as 1 / Mean
=============== 12 passed, 2504 deselected in 199.34s (0:03:19) ================

@Cito
Copy link
Member

Cito commented Apr 2, 2022

Wow, @valgarf, that's a huge PR. I think this is something that can be considered for v3.3, maybe together with supporting GraphQL.js 17. The support for Python 3.6 would then be no problem, as I plan to drop it in the next minor release anyway.

But I'm currently not sure about the advantages and implications of this PR, will need some time to get aquainted with trio and anyio, and finding time for this is currently hard for me. I actually prefer to only use standard Python in GraphQL-core - using anyio would introduce an additional dependency that needs to be mangaged and can cause problems, or some day not be supported or well maintained any more. So we need really good reasons to do this. Also, we need to make sure that there are no performance drawbacks or other issues. There are some benchmarks already, but maybe we need to add some more for testing the consequences of these changes.

My suggestion is that you create an accompanying issue referencing this PR, outlining the advantages a bit more, as an invitation for other users to join the discussion - I would also like to get feedback from existing users regarding this change.

@valgarf valgarf mentioned this pull request Apr 3, 2022
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Cito Cito Awaiting requested review from Cito Cito will be requested when the pull request is marked ready for review Cito is a code owner

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@valgarf @Cito